Skip to content

Conversation

@iromli
Copy link
Contributor

@iromli iromli commented Jan 27, 2026

Prepare


Description

Target issue

closes #13093

Implementation Details


Test and Document the changes

  • Static code analysis has been run locally and issues have been fixed
  • Relevant unit and integration tests have been added/updated
  • Relevant documentation has been updated if any (i.e. user guides, installation and configuration guides, technical design docs etc)

Please check the below before submitting your PR. The PR will not be merged if there are no commits that start with docs: to indicate documentation changes or if the below checklist is not selected.

  • I confirm that there is no impact on the docs due to the code changes in this PR.

Summary by CodeRabbit

Release Notes

  • New Features

    • Added gRPC support for Jans Lock audit service with dedicated routing and backend configuration.
    • Enabled HTTP/2 support for improved protocol negotiation and performance.
  • Configuration Updates

    • Replaced lockEnabled with lockAuditEnabled flag for lock audit endpoint control.
    • Added lockLogLevel and lockLogTarget parameters for lock audit logging configuration.
  • Removed Features

    • Removed Keycloak SAML (kc-saml) plugin support.
    • Disabled Shibboleth component in all-in-one deployments.
  • Changes

    • Consolidated logging configuration for improved maintainability.
    • Updated auth server routing paths and upstream backends.

Signed-off-by: iromli <isman.firmansyah@gmail.com>
@iromli iromli self-assigned this Jan 27, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 27, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR adds gRPC bridge support for the jans-lock plugin, updating logging configuration, restructuring gateway routing, changing web app context paths from /jans-auth to /, and removing Keycloak/SAML dependencies across Helm charts and Docker images.

Changes

Cohort / File(s) Summary
Helm Chart Configuration
charts/janssen-all-in-one/README.md, charts/janssen-all-in-one/values.yaml, charts/janssen/README.md, charts/janssen/values.yaml
Added lockLogLevel and lockLogTarget to auth-server logging configuration; replaced lockEnabled with lockAuditEnabled for audit endpoint enablement; removed Keycloak-related config keys (kc-saml, kcAdminPassword, etc.); added Shibboleth logging configuration.
Kubernetes Service Definitions
charts/janssen-all-in-one/templates/service.yml, charts/janssen/charts/auth-server/templates/service.yml
Added new gRPC Service resources (port 50051) conditionally created when lockEnabled and lockAuditEnabled are both true; mirrors primary service metadata, annotations, and selector patterns.
Kubernetes Deployment & Gateway API
charts/janssen-all-in-one/templates/deployment.yml, charts/janssen-all-in-one/templates/gateway-api.yaml, charts/janssen/templates/gateway-api.yaml
Updated environment variable indentation in deployment; removed Jans Lock and SAML routes from gateway API; added new ROUTE 4 for gRPC apps with conditional HTTPRoute (Istio) or GRPCRoute support; wired gRPC backend on port 50051 with method-based matches for AuditService.
Auth Server Configuration & Scripts
charts/janssen/charts/config/templates/configmaps.yaml, docker-jans-auth-server/Dockerfile, docker-jans-auth-server/scripts/bootstrap.py, docker-jans-auth-server/scripts/upgrade.py
Added string replacements for lock logging targets/levels in configmaps; updated JANS_SOURCE_VERSION commit hash; added http2c module to Jetty; introduced _lock_libs directory for jans-lock JARs; replaced copy_builtin_libs() with copy_lock_libs(); added grpcConfiguration.serverMode: "bridge" to dynamic config.
Auth Server Runtime & Logging
docker-jans-auth-server/scripts/entrypoint.sh, docker-jans-auth-server/scripts/lock.py, docker-jans-auth-server/scripts/healthcheck.py, docker-jans-auth-server/templates/jans-auth/log4j2.xml, docker-jans-auth-server/templates/jans-lock/log4j2-lock.xml
Replaced dynamic log4j2 configuration detection with fixed path (resources/log4j2.xml); removed configure_lock_logging() function; changed health-check endpoint path from /jans-auth/sys/health-check to /sys/health-check; expanded log4j2.xml with comprehensive Appenders/Loggers sections including JANS_LOCK_FILE appender; deleted separate log4j2-lock.xml file.
Web App Context & Jetty
docker-jans-auth-server/templates/jans-auth/jans-auth.xml
Changed Jetty WebAppContext path from /jans-auth to /, altering the URL namespace under which the WAR is served.
Nginx Configuration
docker-jans-all-in-one/app/templates/nginx/nginx-default.conf, docker-jans-all-in-one/app/templates/nginx/jans-auth-upstream.conf, docker-jans-all-in-one/app/templates/nginx/jans-auth-location.conf
Enabled HTTP/2 support in nginx server block; added new jans_auth_grpc_backend upstream pointing to 127.0.0.1:8081; removed /jans-auth path prefix in proxy_pass directives for multiple endpoints; added new gRPC location block for /io.jans.lock.audit.AuditService/ with grpc_pass and standard headers.
Docker Images & Build
docker-jans-all-in-one/Dockerfile, docker-jans-persistence-loader/Dockerfile, docker-jans-persistence-loader/scripts/hooks.py
Commented out JANS_SHIBBOLETH_IMAGE build stage and removed Shibboleth-related asset copies; updated JANS_SOURCE_VERSION across images; added disableExternalLoggerConfiguration default key to persistence loader configuration hook.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • #12949 — Implements the same feature with gRPC audit endpoints and complete supporting artifacts across Helm, Docker, and runtime components.
  • #13090 — Modifies the jans-lock/jans-auth dependency extraction and library placement flow with matching changes to custom libs wiring.
  • #13106 — Enables jans-lock gRPC bridge by default through grpcConfiguration configuration additions.

Suggested reviewers

  • iromli
  • yurem
  • yuriyz
  • devrimyatar
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title 'feat(cloud-native): add support for gRPC bridge' clearly and concisely describes the main change—adding gRPC bridge support to cloud-native deployments, which aligns with the changeset.
Description check ✅ Passed The PR description addresses the target issue (#13093) and confirms documentation updates and code analysis completion. However, the Implementation Details section is empty, and unit/integration tests were not added/updated.
Linked Issues check ✅ Passed The PR successfully implements the objective from issue #13093 to enable gRPC bridge mode for the jans-lock plugin via http2c module across Dockerfiles, Helm charts, and nginx configurations.
Out of Scope Changes check ✅ Passed Changes are within scope: gRPC bridge support for jans-lock, lock audit logging, and lock endpoint enablement. Shibboleth Dockerfile changes appear unrelated but are minimal config updates outside primary objectives.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch cn-grpc-bridge

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@mo-auto mo-auto added comp-docker-jans-auth-server kind-feature Issue or PR is a new feature request labels Jan 27, 2026
@mo-auto
Copy link
Member

mo-auto commented Jan 27, 2026

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

iromli and others added 15 commits January 28, 2026 14:31
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
@iromli iromli marked this pull request as ready for review February 5, 2026 21:43
@iromli iromli requested a review from moabu as a code owner February 5, 2026 21:43
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

🤖 Fix all issues with AI agents
In `@charts/janssen-all-in-one/README.md`:
- Line 64: Update the README entry for the configuration key
auth-server.appLoggers.auditStatsLogTarget so its description correctly reflects
that it configures the audit stats log target (use "jans-auth_audit.log target")
instead of the incorrect "jans-auth_script.log target"; locate the table row
containing auth-server.appLoggers.auditStatsLogTarget and replace the
description text to "jans-auth_audit.log target" to match the key name and the
neighboring Line 63 entry.
- Line 192: Update the kc-scheduler README entry to explicitly state that
kc-scheduler is only deployed/usable when saml.enabled is true and reference the
template condition used to enforce this ({{ if and (index .Values "kc-scheduler"
"enabled") (.Values.saml.enabled) }}); add a short note explaining what the SAML
service provides (Keycloak SAML clients and dependencies) and how it interacts
with config-api.plugins (e.g., that config-api.plugins may include saml-related
entries but kc-scheduler specifically requires saml.enabled: true), so users
understand the dependency and default behavior.

In `@charts/janssen-all-in-one/templates/gateway-api.yaml`:
- Around line 446-515: The ROUTE 4 GRPCRoute/HTTPRoute block (metadata name {{
$fullName }}-grpc-routes) can emit an empty spec.rules when the lock audit
condition is false; wrap the entire ROUTE 4 section (from the apiVersion line
through the end of this route) inside the same conditional used for lock audit
e.g. the if and (index .Values "auth-server" "lockEnabled") (index .Values
"auth-server" "ingress" "lockAuditEnabled") check so spec.rules is only rendered
when those conditions are true, and close the conditional after the existing
route content to prevent producing an empty rules: entry.

In `@charts/janssen/templates/gateway-api.yaml`:
- Around line 429-498: The GRPC route resource (metadata name {{ $fullName
}}-grpc-routes) can emit an empty spec.rules when lock audit is disabled; wrap
the entire ROUTE 4 resource block (from apiVersion/kind/metadata/spec through
the end of that section) in the same conditional that currently gates the inner
rules (the check using .Values.global "auth-server" "lockEnabled" and
.Values.global "auth-server" "ingress" "lockAuditEnabled") so the resource is
not rendered at all when those flags are false, ensuring no empty rules: field
is produced for HTTPRoute/GRPCRoute.

In `@docker-jans-all-in-one/app/templates/nginx/jans-auth-location.conf`:
- Around line 201-211: The location block for gRPC (location
/io.jans.lock.audit.AuditService/) incorrectly includes the proxy_http_version
1.1 directive which is only valid for the proxy_pass module; remove the
proxy_http_version line from this gRPC location so nginx uses its grpc_pass
handling (grpc_pass grpc://jans_auth_grpc_backend) and HTTP/2 is negotiated by
the gRPC module natively.

In `@docker-jans-all-in-one/Dockerfile`:
- Around line 286-289: Remove the leftover malformed continuation comments by
deleting the two lines containing "# \" and "# /opt/idp" (they follow the
"/usr/share/java" line) so there is no confusing broken-line continuation in the
Dockerfile; if a line continuation was intended for a chown or COPY target,
instead add a proper trailing backslash to the actual preceding line (e.g.,
after "/usr/share/java") and ensure the continuation line contains the real path
rather than commented text.
- Around line 325-328: Remove the orphaned commented lines ("# \\" and "#
/opt/idp") that follow the "/usr/share/java" entry in the Dockerfile and mirror
the same cleanup applied to the chown block so the chmod command block is clean
and has no stray commented lines; locate the chmod/chown sequence around the
"/usr/share/java" token and delete those leftover comment lines so the chmod
command is contiguous and readable.

In `@docker-jans-auth-server/Dockerfile`:
- Around line 95-100: The RUN block that downloads and unpacks jans-lock-server
deps contains an orphaned commented line ("&& rm -f
${JETTY_BASE}/jans-auth/custom/libs/grpc-bridge-runtime-*.jar") which breaks
readability and could accidentally be misjoined; either remove that dead comment
or incorporate it correctly into the RUN chain. Edit the Dockerfile RUN that
uses CN_VERSION and JETTY_BASE so it ends consistently: either delete the
commented rm line entirely, or restore it as a real chained command (add the
leading && before rm and ensure the previous line ends with a backslash) so the
cleanup of ${JETTY_BASE}/jans-auth/custom/libs/grpc-bridge-runtime-*.jar is
executed as intended.

In `@docker-jans-auth-server/scripts/upgrade.py`:
- Around line 83-102: The env var CN_LOCK_GRPC_PORT is being treated as a string
(grpc_port) causing wrong comparisons and string overwrites; update the upgrade
logic in upgrade.py so grpc_port is parsed to an int once (with
validation/fallback to default 8080 if non-numeric) and use that integer for
both creating conf["grpcConfiguration"]["grpcPort"] and for
comparisons/assignments in the update loop (i.e., compare
conf["grpcConfiguration"].get("grpcPort", 8080) to the parsed int and assign the
parsed int), ensuring no ValueError bubbles up on invalid env input.

In `@docker-jans-auth-server/templates/jans-auth/log4j2.xml`:
- Around line 78-85: The JANS_LOCK_FILE RollingFile appender currently only uses
SizeBasedTriggeringPolicy and a different PatternLayout; update the
JANS_LOCK_FILE RollingFile to add a TimeBasedTriggeringPolicy (daily) alongside
the existing SizeBasedTriggeringPolicy so logs rotate by date and size
consistent with other appenders, and align the PatternLayout to match the
standard layout (use the same timestamp format and include the MDC field
%X{X-Correlation-Id} for tracing); if the differences are intentional for
lock-specific behavior, add a brief comment above the RollingFile explaining the
rationale instead.

Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
docker-jans-auth-server/scripts/upgrade.py (1)

31-34: ⚠️ Potential issue | 🟡 Minor

Duplicate hostname assignment — line 31 is dead code.

hostname is assigned on line 31 and immediately reassigned on line 34. The first assignment is unused.

Proposed fix
 def _transform_lock_dynamic_config(conf, manager):
     should_update = False

-    hostname = manager.config.get("hostname")
-
     # add missing top-level keys
     hostname = manager.config.get("hostname")
docker-jans-all-in-one/Dockerfile (1)

223-226: 🧹 Nitpick | 🔵 Trivial

Shibboleth env vars are still defined despite the feature being disabled.

CN_SHIBBOLETH_HTTP_HOST, CN_SHIBBOLETH_HTTP_PORT, CN_SHIBBOLETH_JAVA_OPTIONS, and SHIBBOLETH_HOME are still set, but the shibboleth build stage and all COPY commands are commented out. SHIBBOLETH_HOME=/opt/shibboleth-idp references a path that won't exist in the image. This is harmless for now, but if any script references SHIBBOLETH_HOME at runtime, it would point to a non-existent directory.

Consider commenting these out as well for consistency with the disabled state, or adding a note that they're retained intentionally.

🤖 Fix all issues with AI agents
In `@charts/janssen-all-in-one/README.md`:
- Line 96: Update the README table entry for the auth-server flag
auth-server.ingress.lockAuditEnabled to use the imperative "Enable endpoint
/io.jans.lock.audit.AuditService" instead of "Enabled endpoint …" so it matches
the style of other entries (e.g., the /.well-known/lock-server-configuration
row); edit the table cell text accordingly in the README.md.

In `@charts/janssen-all-in-one/templates/gateway-api.yaml`:
- Around line 446-448: The YAML document separator '---' is placed before the
Helm conditional (the line with {{- if and (index .Values "auth-server"
"lockEnabled") (index .Values "auth-server" "ingress" "lockAuditEnabled") }})
causing an empty document when both lockEnabled and lockAuditEnabled are false;
move the '---' inside that if block (i.e., place the separator immediately after
the opening {{- if ... }} so it's only rendered when the condition is true) to
avoid emitting a trailing empty YAML document.
- Around line 453-459: The template currently emits GRPCRoute (or HTTPRoute)
based only on .Values.gatewayApi.gatewayClassName; add a values field (e.g.,
gatewayApi.apiVersion) and protect rendering of GRPCRoute by checking that the
configured/expected Gateway API version is v1 (or >= v1.1) before emitting kind:
GRPCRoute, otherwise either emit a safe alternative (HTTPRoute) or fail
rendering with a clear error message; update the template logic in
gateway-api.yaml (the conditional that chooses kind) to reference
gatewayApi.apiVersion and adjust README/values documentation to state the
minimum required Gateway API version (v1.1+/v1) so cluster compatibility is
explicit.
- Around line 488-489: The gRPC port 50051 is hardcoded in gateway-api.yaml (the
backendRefs and the port block using name {{ $svcName }}-grpc) and duplicated in
service.yml, causing possible mismatches; add a configurable value (e.g.,
values.yaml key service.grpcPort) and replace the literal 50051 in
gateway-api.yaml (the port entry and any backendRefs that reference the gRPC
port name) and in service.yml to use that value (templating the port with
.Values.service.grpcPort) so both templates reference the same centralized
setting.

In `@docker-jans-all-in-one/app/templates/nginx/jans-auth-location.conf`:
- Around line 201-210: Add gRPC-specific timeouts to the AuditService location
block to prevent long-lived streaming RPCs from being cut off: in the location
/io.jans.lock.audit.AuditService/ (the gRPC proxy block that sets
grpc_set_header and grpc_pass to jans_auth_grpc_backend) add grpc_read_timeout
(e.g. 1h or an appropriate long value) and grpc_send_timeout (matching read
timeout for long streams) and set grpc_connect_timeout to a short value (e.g.
30s) so connects fail fast; keep values configurable if needed.

In `@docker-jans-auth-server/Dockerfile`:
- Around line 95-99: The Dockerfile currently unconditionally unpacks the
jans-lock service-deps into ${JETTY_BASE}/jans-auth/custom/libs which exposes
lock dependencies even when CN_LOCK_ENABLED is false; change the Dockerfile to
only download/store the jans-lock-server-${CN_VERSION}-service-deps-pack.zip (or
place it into ${JETTY_BASE}/jans-auth/_libs) without unpacking it, and update
the bootstrap logic that implements copy_builtin_libs() in
docker-jans-auth-server/scripts/bootstrap.py to conditionally unpack or copy
that service-deps-pack into the runtime classpath only when CN_LOCK_ENABLED is
true (mirror how core lock JARs are handled), ensuring service-deps-pack is
referenced by its file name and paths (${CN_VERSION}, service-deps-pack,
${JETTY_BASE}/jans-auth/_libs, and copy_builtin_libs()) so the change is easy to
locate.

In `@docker-jans-auth-server/scripts/upgrade.py`:
- Around line 97-101: The current except block for converting grpc_port uses
int(jetty_port) directly and an f-string logger (Ruff G004); update it to first
log with lazy % formatting (use logger.warning with "%s" placeholders
referencing grpc_port and jetty_port), then attempt to convert jetty_port inside
its own try/except; if int(jetty_port) succeeds assign that to grpc_port,
otherwise log a second warning (again using % formatting) and set grpc_port to a
safe numeric fallback (e.g., 8080). Refer to the grpc_port and jetty_port
variables and the logger instance to find and modify the conversion and logging
logic in upgrade.py.
- Around line 111-115: The comparison loop uses a hardcoded 8080 for the
"grpcPort" old-default, which mismatches the create-path default derived from
jetty_port; update the tuple for ("grpcPort", 8080, grpc_port) to use the
computed default (grpc_port or jetty_port-derived value) instead of literal 8080
so the update path and create path share the same default; locate the tuple in
the for attr in [...] block and replace the 8080 literal with the grpc_port
variable (or the jetty_port-based default) to ensure consistent behavior.

Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
docker-jans-auth-server/scripts/upgrade.py (1)

31-34: 🧹 Nitpick | 🔵 Trivial

Duplicate hostname assignment.

Line 34 re-assigns hostname = manager.config.get("hostname") which is already assigned on line 31. One of these is redundant.

Remove duplicate assignment
     should_update = False

     hostname = manager.config.get("hostname")

     # add missing top-level keys
-    hostname = manager.config.get("hostname")
     for missing_key, value in [
docker-jans-auth-server/scripts/bootstrap.py (1)

179-192: 🧹 Nitpick | 🔵 Trivial

Use lazy %-formatting in logger.warning().

Ruff G004: f-string in logging statement on lines 185-186. Use %-style formatting consistent with Python logging best practices.

Proposed fix
     if not lock_jars:
         logger.warning(
-            f"Required JAR files to run jans-lock plugin were not found in {libs_path}. "
-            "The plugin may not run properly and affect jans-auth server."
+            "Required JAR files to run jans-lock plugin were not found in %s. "
+            "The plugin may not run properly and affect jans-auth server.",
+            libs_path,
         )
         return
charts/janssen/templates/gateway-api.yaml (1)

68-78: ⚠️ Potential issue | 🔴 Critical

Public well-known routes require a URLRewrite filter to prepend the /jans-auth/ context path.

The auth-server's OpenID configuration servlet is mapped at /.well-known/openid-configuration, but when deployed in the non-AIO chart at context /jans-auth, the full path becomes /jans-auth/.well-known/openid-configuration.

ROUTE 1 (lines 68–227) lacks URLRewrite filters for public endpoints (OpenID config, device code, UMA configuration, FIDO configuration, Webfinger, Simple Web Discovery), while ROUTE 2 (lines 276–291) correctly uses ReplacePrefixMatch: //jans-auth/ to add the context path. Without these filters, ROUTE 1 traffic will fail to reach the auth-server. Apply the same URLRewrite pattern used in ROUTE 2 to all public routes in ROUTE 1.

🤖 Fix all issues with AI agents
In `@docker-jans-auth-server/Dockerfile`:
- Around line 70-73: Add explicit creation of the missing custom/_lock_libs
directory to the initial mkdir -p call so it cannot be missing at runtime;
update the mkdir invocation that currently lists ${JETTY_BASE}/jans-auth/_libs,
${JETTY_BASE}/jans-auth/_lock_libs and ${JETTY_BASE}/jans-auth/custom/libs to
also include ${JETTY_BASE}/jans-auth/custom/_lock_libs (referencing the
JETTY_BASE variable and the jans-auth/custom/_lock_libs path) to ensure the
folder exists even if the later unzip/download step produces no files.
- Around line 88-99: The copy_lock_libs() bootstrap.py logic only handles
_lock_libs JARs but ignores service dependencies staged at custom/_lock_libs by
the Dockerfile; update copy_lock_libs() to, when CN_LOCK_ENABLED=true, also copy
contents of ${JETTY_BASE}/jans-auth/custom/_lock_libs into the runtime classpath
destination (e.g., ${JETTY_BASE}/jans-auth/custom/libs/) or alternatively ensure
custom/_lock_libs is only added to the classpath conditionally — modify the
function to detect and copy files from custom/_lock_libs to custom/libs (or
conditionally include custom/_lock_libs in the classpath) and add a
unit/boot-time check so service deps are not loaded when CN_LOCK_ENABLED=false.

In `@docker-jans-auth-server/scripts/bootstrap.py`:
- Around line 86-90: Read the raw env var first and convert it to boolean in two
steps (avoid the walrus evaluating a default non-empty string): raw_lock =
os.environ.get("CN_LOCK_ENABLED") then lock_enabled = as_boolean(raw_lock) (or
lock_enabled = as_boolean(raw_lock) if raw_lock is not None else False); use
that lock_enabled in the if statement to guard copy_lock_libs() (function name:
copy_lock_libs) and change the logger call to use lazy %-style logging:
logger.info("The jans-lock plugin is enabled via CN_LOCK_ENABLED=%s",
lock_enabled) so the boolean value is meaningful and Ruff G004 is satisfied.

Signed-off-by: iromli <isman.firmansyah@gmail.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docker-jans-auth-server/scripts/bootstrap.py (1)

179-192: 🧹 Nitpick | 🔵 Trivial

Use lazy %-formatting in logger.warning to satisfy Ruff G004.

Line 185 uses an f-string inside logger.warning(), which Ruff G004 flags. This is consistent with how the rest of the codebase's logging calls should behave — deferring string interpolation to the logging framework.

Proposed fix
     if not lock_jars:
         logger.warning(
-            f"Required JAR files to run jans-lock plugin were not found in {libs_path}. "
-            "The plugin may not run properly and affect jans-auth server."
+            "Required JAR files to run jans-lock plugin were not found in %s. "
+            "The plugin may not run properly and affect jans-auth server.",
+            libs_path,
         )
         return

coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 6, 2026
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docker-jans-auth-server/scripts/upgrade.py (1)

31-34: ⚠️ Potential issue | 🟡 Minor

Duplicate hostname assignment.

Line 34 is an exact duplicate of line 31. Remove one of them.

Proposed fix
 def _transform_lock_dynamic_config(conf, manager):
     should_update = False

     hostname = manager.config.get("hostname")

     # add missing top-level keys
-    hostname = manager.config.get("hostname")
     for missing_key, value in [
🤖 Fix all issues with AI agents
In `@docker-jans-auth-server/scripts/upgrade.py`:
- Line 41: The migration currently sets a default protectionMode of "oauth" but
only converts legacy "CEDARLING"→"cedarling" in the migration block, leaving
existing protectionMode="cedarling" unchanged; update the migration logic in
upgrade.py (the block handling CEDARLING → cedarling around the CEDARLING
symbol) to also transform "cedarling" → "oauth" (or replace the two-step
conversion by mapping "CEDARLING" directly to "oauth") so all existing installs
with either "CEDARLING" or "cedarling" end up as "oauth".

coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 6, 2026
Signed-off-by: Mohammad Abudayyeh <47318409+moabu@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@charts/janssen/README.md`:
- Line 346: Update the README entry for the Helm value
global.auth-server.ingress.lockAuditEnabled to explicitly mention that
/io.jans.lock.audit.AuditService is a gRPC endpoint (e.g., "Enable gRPC endpoint
/io.jans.lock.audit.AuditService") so users know the ingress/bridge handles gRPC
traffic; locate the table row containing
global.auth-server.ingress.lockAuditEnabled and replace or augment the
description accordingly.

| global.auth-server.ingress.firebaseMessagingEnabled | bool | `true` | Enable endpoint /firebase-messaging-sw.js |
| global.auth-server.ingress.firebaseMessagingLabels | object | `{}` | Firebase Messaging ingress resource labels. key app is taken |
| global.auth-server.ingress.lockAdditionalAnnotations | object | `{}` | Lock ingress resource additional annotations. |
| global.auth-server.ingress.lockAuditEnabled | bool | `false` | Enable endpoint /io.jans.lock.audit.AuditService |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider clarifying that this is a gRPC endpoint.

The description "Enable endpoint /io.jans.lock.audit.AuditService" is technically accurate, but users might benefit from explicitly stating this is a gRPC endpoint, especially since the PR introduces gRPC bridge support.

📝 Suggested description improvement
-| global.auth-server.ingress.lockAuditEnabled | bool | `false` | Enable endpoint /io.jans.lock.audit.AuditService |
+| global.auth-server.ingress.lockAuditEnabled | bool | `false` | Enable gRPC endpoint /io.jans.lock.audit.AuditService |
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
| global.auth-server.ingress.lockAuditEnabled | bool | `false` | Enable endpoint /io.jans.lock.audit.AuditService |
| global.auth-server.ingress.lockAuditEnabled | bool | `false` | Enable gRPC endpoint /io.jans.lock.audit.AuditService |
🤖 Prompt for AI Agents
In `@charts/janssen/README.md` at line 346, Update the README entry for the Helm
value global.auth-server.ingress.lockAuditEnabled to explicitly mention that
/io.jans.lock.audit.AuditService is a gRPC endpoint (e.g., "Enable gRPC endpoint
/io.jans.lock.audit.AuditService") so users know the ingress/bridge handles gRPC
traffic; locate the table row containing
global.auth-server.ingress.lockAuditEnabled and replace or augment the
description accordingly.

Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(cloud-native): add support for gRPC bridge

3 participants